-
Notifications
You must be signed in to change notification settings - Fork 609
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
api: add BindOptions.NonRecursive #2780
Conversation
bca09de
to
8175894
Compare
Codecov Report
@@ Coverage Diff @@
## master #2780 +/- ##
=========================================
Coverage ? 61.94%
=========================================
Files ? 137
Lines ? 22131
Branches ? 0
=========================================
Hits ? 13709
Misses ? 6947
Partials ? 1475 |
@dperny PTAL? |
LGTM. Sorry, was on vacation last week. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @AkihiroSuda - could you update the other dependencies as well? (see below)
8175894
to
70f9ded
Compare
From moby/moby#38003 Signed-off-by: Akihiro Suda <suda.akihiro@lab.ntt.co.jp>
70f9ded
to
5338701
Compare
updated |
@@ -9175,6 +9192,10 @@ file { | |||
name: "ConfChangeUpdateNode" | |||
number: 2 | |||
} | |||
value { | |||
name: "ConfChangeAddLearnerNode" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was added in etcd-io/etcd#8751, related to etcd-io/etcd#8568
wondering if we need local changes to take the new type into account (e.g. #1711)
(should be ok as follow up)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @dperny
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but left one comment for a possible follow-up
Going back, I'm confused as to why there are so many vendoring updates included with this PR |
These are just updated to match the versions that are used with the current version of moby/moby. Train of thought there is that CI in this repository will be testing against the same versions of the dependencies than it will ultimately be used with when vendoring swarmkit in moby/moby. See #2780 (comment) above; the initial version of this PR only updated the moby/moby vendor itself (and CI was green with that) |
Alright, looks good to me. |
Sorry to everyone who's PRs will now be broken and need a rebase. |
Don't think there's other PR's doing a vendor update, so we should be fine |
From moby/moby#38003
Signed-off-by: Akihiro Suda suda.akihiro@lab.ntt.co.jp